feat: 학생 회원가입 인증 로직 변경 #669
Conversation
| @Id | ||
| @Indexed | ||
| private String email; | ||
|
|
||
| @Indexed | ||
| private String authToken; | ||
|
|
||
| @Indexed | ||
| private String nickname; |
| @RedisHash(value = "StudentTemporaryStatus") | ||
| public class StudentTemporaryStatus { | ||
|
|
||
| private static final long CACHE_EXPIRE_SECOND = 60 * 60 * 10L; |
There was a problem hiding this comment.
A
저번 레거시부터 느꼈지만, 굳이 회원가입 인증 토큰을 10시간 보관하는게 괜찮은가 싶네요
10시간동안 안 받을 정도면..🤔
There was a problem hiding this comment.
그러게요... 근데 수정하게 된다면 팀 전체 논의사항으로 올려야 할것같네요..
There was a problem hiding this comment.
지금 인증번호를 받는 형식이 아니라 저는 괜찮을 수도 있을 것 같아요
일반적으로 인증번호는 시간을 짧게 하고 저렇게 메일로 보내서 접속하면 인증되게 하는 방식은 엄청 길게 주더라구요
(얼마전에 microsoft 관련해서도 까먹고 있다가 6개월 만에 가서 받았는데 됐음)
| public static StudentTemporaryStatus of(StudentRegisterRequest request, String authToken) { | ||
| return new StudentTemporaryStatus(request.email(), authToken, request.nickname(),request.name(), request.password(), request.gender(), | ||
| request.isGraduated(), request.department(), request.studentNumber(), request.phoneNumber()); | ||
| } |
|
|
||
| StudentTemporaryStatus save(StudentTemporaryStatus studentTemporaryStatus); | ||
|
|
||
| Optional<StudentTemporaryStatus> findById(String email); |
There was a problem hiding this comment.
C
findByEmail이 낫지 않을까요?
| Optional<StudentTemporaryStatus> findById(String email); | |
| Optional<StudentTemporaryStatus> findByEmail(String email); |
There was a problem hiding this comment.
저도 처음에 findByEmail로 했는데, 현재 key값인 email은 Id로 해야만 데이터를 찾을 수 있더라고요 ㅋㅋ
| Optional<StudentTemporaryStatus> findByAuthToken(String authToken); | ||
|
|
||
| void deleteById(String email); | ||
| } |
| if (student.getUser().getNickname() != null) { | ||
| userRepository.findByNickname(student.getUser().getNickname()) | ||
| private void validateDataExist(StudentRegisterRequest request) { | ||
| userRepository.findByEmail(request.email()) |
| @@ -51,12 +55,13 @@ public class UserService { | |||
| @Transactional | |||
| public UserLoginResponse login(UserLoginRequest request) { | |||
| User user = userRepository.getByEmail(request.email()); | |||
| Optional<StudentTemporaryStatus> studentTemporaryStatus = studentRedisRepository.findById(request.email()); | |||
There was a problem hiding this comment.
C
이 로직의 경우는 getByEmail을 사용해서 예외처리 해주는 건 어떨까요?
There was a problem hiding this comment.
일반적으로 Repository에서 getBy를 통해 예외처리를 하는 상황은 "데이터를 찾을 수 없습니다."라는 일반적인 예외를 발생시킬때 사용하는것같습니다. 하지만 현재 "미인증 상태입니다. 아우누리에서 인증메일을 확인해주세요"라는 예외처리를 내야 하는 상황인데 이걸 Repository에 구현하는건 너무 사소한 부분까지 Repository계층에서 책임을 맡게되는것같아 계층의 경계가 모호해지는것같아서 서비스계층에서 예외처리 했습니다. 이 부분에 대해 혹시 어덯게 생각하시나요!
There was a problem hiding this comment.
Repository에서 이를 처리한다면, 혹시나 나중에 findById를 쓰게 되는 메소드가 생긴다면 문제가 생길 수도 있겠네요.
코드는 확장성을 고려해서 짜는 것이 중요하니까요..!
저는 혹시나 id를 못 찾는 경우를 생각해서 예외 관련해서 말씀드려봤답니다🤔
| @@ -121,4 +121,4 @@ void delete() { | |||
| } | |||
| dataInitializer.clear(); | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
AcceptanceTest는 왜 커밋에 올라간거지....ㅋㅋ 일단 고치겠습니다 감사합니다!
src/test/resources/application.yml
Outdated
| @@ -72,4 +72,4 @@ naver: | |||
|
|
|||
| OPEN_API_KEY_PUBLIC: testck | |||
|
|
|||
| OPEN_API_KEY_TMONEY: testck | |||
| OPEN_API_KEY_TMONEY: testck | |||
| @@ -51,12 +55,13 @@ public class UserService { | |||
| @Transactional | |||
| public UserLoginResponse login(UserLoginRequest request) { | |||
| User user = userRepository.getByEmail(request.email()); | |||
| Optional<StudentTemporaryStatus> studentTemporaryStatus = studentRedisRepository.findById(request.email()); | |||
There was a problem hiding this comment.
Repository에서 이를 처리한다면, 혹시나 나중에 findById를 쓰게 되는 메소드가 생긴다면 문제가 생길 수도 있겠네요.
코드는 확장성을 고려해서 짜는 것이 중요하니까요..!
저는 혹시나 id를 못 찾는 경우를 생각해서 예외 관련해서 말씀드려봤답니다🤔
| @RedisHash(value = "StudentTemporaryStatus") | ||
| public class StudentTemporaryStatus { | ||
|
|
||
| private static final long CACHE_EXPIRE_SECOND = 60 * 60 * 10L; |
There was a problem hiding this comment.
지금 인증번호를 받는 형식이 아니라 저는 괜찮을 수도 있을 것 같아요
일반적으로 인증번호는 시간을 짧게 하고 저렇게 메일로 보내서 접속하면 인증되게 하는 방식은 엄청 길게 주더라구요
(얼마전에 microsoft 관련해서도 까먹고 있다가 6개월 만에 가서 받았는데 됐음)
인증시간이 지나도 보조인덱스인 @indexed 필드는 자동삭제 안되는 문제였습니다., 근데 지금보니 그 부분 해결하는걸 깜빡했네요; 감사합니다..큰일날뻔했네요 |
🔥 연관 이슈
현재 학생이 회원가입 후 이메일 인증을 안해도 여전히 Mysql에 학생회원 정보가 남아있는 상황입니다.
그렇기때문에 인증시간이 지나고 다시 회원가입할때 이메일이나 닉네임 중복 에러가 발생되고있습니다.
🚀 작업 내용
변경된 로직은 아래와 같이 이루어집니다.
학생이 회원가입을 하면 Redis에 정보를 10시간동안 저장합니다.
(회원가입할때 mysql, redis둘다 이메일, 닉네일 중복 검사)
이메일 인증 시 Redis에서 인증 토큰을 검사합니다.
인증이 정상적으로 이루어지면 mysql저장 후 redis에서 데이터 삭제합니다.
또한 인증 토큰 관련 오류메시지중 "이미 만료된 토큰입니다.", "이미 인증된 사용자입니다.", "토큰에 해당하는 사용자를 찾을 수 없습니다." 라는 오류메시지 출력을 "유효하지 않는 토큰입니다." 로 간소화했습니다.
Redis관련 이해를 돕기위해 아래와같이 사진으로 과정 첨부합니다.
회원가입시 Redis저장합니다.
중복된 이메일로 회원가입시 Redis에서 중복검증합니다.
회원가입 인증시 Redis에 저장된 데이터를 삭제합니다.
Redis관련작업은 아래와 같은 공식문서에서 참조해서 작업했습니다.
https://docs.spring.io/spring-data/redis/reference/redis/redis-repositories/indexes.html
https://redis.io/docs/latest/integrate/redisom-for-java/
💬 리뷰 중점사항